reference_to_pose function #19
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
+ Coverage 97.86% 98.03% +0.17%
==========================================
Files 4 5 +1
Lines 374 407 +33
Branches 72 87 +15
==========================================
+ Hits 366 399 +33
Misses 4 4
Partials 4 4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Andeshog
left a comment
Andeshog
left a comment
There was a problem hiding this comment.
Add some tests also and I will be very happy
| template <typename T> | ||
| geometry_msgs::msg::Pose reference_to_pose(const T& ref) { | ||
| geometry_msgs::msg::Pose pose; | ||
| pose.position.x = ref.x; | ||
| pose.position.y = ref.y; | ||
| pose.position.z = ref.z; | ||
|
|
||
| tf2::Quaternion q; | ||
| q.setRPY(ref.roll, ref.pitch, ref.yaw); | ||
| pose.orientation = tf2::toMsg(q); | ||
|
|
||
| return pose; | ||
| } |
There was a problem hiding this comment.
Could use a simple concept to constrain the input type in a better way. Something like
template <typename T>
concept PoseLike = requires(const T& t) {
{ t.x } -> std::convertible_to<double>;
{ t.y } -> std::convertible_to<double>;
{ t.z } -> std::convertible_to<double>;
{ t.roll } -> std::convertible_to<double>;
{ t.pitch } -> std::convertible_to<double>;
{ t.yaw } -> std::convertible_to<double>;
};
template <PoseLike T>
geometry_msgs::msg::Pose reference_to_pose(const T& ref) {
geometry_msgs::msg::Pose pose;
pose.position.x = ref.x;
pose.position.y = ref.y;
pose.position.z = ref.z;
tf2::Quaternion q;
q.setRPY(ref.roll, ref.pitch, ref.yaw);
pose.orientation = tf2::toMsg(q);
return pose;
}There was a problem hiding this comment.
Also, I would say it is not strictly necessary to use tf2 here anymore, since we are packing it in a function anyway (so it doesnt hurt if the function is longer and more verbose).
There was a problem hiding this comment.
Since this is purely a ros to ros conversion I don't see why we can't use available ros functions here.
There was a problem hiding this comment.
You can, but why drag in another dependency just because. Also possible to create a separate function that converts Eigen::Quaternion to ros orientation, and then use that directly in reference_to_pose (with euler_to_quat). Also, it isnt really a strict ros to ros conversion, as T can also be Eta from utils here
| #ifndef ROS_CONVERSIONS_HPP | ||
| #define ROS_CONVERSIONS_HPP |
There was a problem hiding this comment.
VORTEX_UTILS_ROS_CONVERSIONS_HPP or something like that
|
The thought was more to test that the template works with for instance |
Andeshog
left a comment
Andeshog
left a comment
There was a problem hiding this comment.
lgtm, just make the test pass 🍇
reference_to_pose is templated for now, to avoid a dependency on vortex_msgs.
Can therefore be called like this: